TL/UCP: add exact mixed-radix allgather schedules by jeffnvidia · Pull Request #1327 · openucx/ucc

jeffnvidia · 2026-07-12T14:13:09Z

What

Add exact per-phase radix schedules to TL/UCP K-nomial AllGather. A schedule
such as 8x6 uses radix 8 in the first phase and radix 6 in the second; its
product must equal the team size.

Set an explicit schedule with, for example:

UCC_TL_UCP_ALLGATHER_KN_RADIX=8x6

The same option accepts a fixed radix or auto. The existing fixed-radix path
and AllGatherV behavior remain unchanged. Invalid schedules are rejected, and
existing algorithm tuning still decides whether K-nomial AllGather is selected.

Why

For non-power team sizes, a fixed radix can produce a truncated final phase or
proxy/extra ranks. An exact mixed schedule covers the complete team with an
explicit radix for every phase.

Across three tested environments at 96 ranks, a representative exact schedule
improved the large-message endpoint by 1.81x--1.99x over the best measured
fixed radix by avoiding the fixed-radix remainder path.

Performance evidence

Results are intentionally anonymized and normalized to the best measured
fixed-radix execution for each environment. Each row uses one exact schedule
at the endpoint; it is not a pointwise best-schedule envelope.

Environment	Team	Exact schedule	Fixed baseline	Normalized speedup
System A	96 ranks	`4x4x6`	R4	1.98x
System B	96 ranks	`2x2x2x2x2x3`	R4	1.99x
System C	96 ranks	`8x3x4`	R3	1.81x

These relative results demonstrate the remainder-path mechanism; they are not
intended as absolute system-performance comparisons.

Validation

Capability debug build and focused schedule tests passed.
16-rank CUDA AllGather covered fixed R2/R4/R8 and explicit 8x2, 2x8,
and 4x4; all cases passed with zero failures.
Tests cover parsing and bounded formatting, legacy fixed-radix behavior, and
exact mixed peer sets, segment sizes, and offsets for every rank.

Automatic schedule selection is proposed separately in #1328.

jeffnvidia · 2026-07-15T12:55:25Z

Maintainer note on the tl_ucp.c diff and codestyle:

I intentionally reduced this file to the functional seven-line
ALLGATHER_KN_MIXED_RADICES entry and removed the unrelated formatting churn.
With clang-format 21.1.8, touching that entry expands formatting to the entire
legacy configuration-array initializer, changing the file from 7 additions / 0
deletions to 105 additions / 49 deletions. I did not add formatter-suppression
directives.

The minimal diff may therefore make the codestyle check fail. Could maintainers
please accept/waive that result, or advise if a separate formatting-only change
is preferred?

Add explicit per-phase radix schedules to TL/UCP K-nomial AllGather while preserving the fixed-radix and AllGatherV paths. Parse and validate exact schedules through the existing team configuration.

Cover schedule parsing, legacy fixed-radix behavior, and exact mixed-radix peer and segment layouts.

greptile-apps · 2026-08-17T06:02:54Z

Greptile Summary

The PR adds exact mixed-radix schedules for TL/UCP K-nomial AllGather while retaining the fixed-radix and AllGatherV paths.

Adds parsing, formatting, cloning, and range lookup for ordered radix schedules.
Extends K-nomial pattern progression to use a per-phase radix.
Validates that mixed-schedule products equal the team size.
Adds focused parser and peer-layout tests.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains within the eligible follow-up scope.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/components/tl/ucp/allgather/allgather_knomial.c	Selects and validates message-range radix schedules and initializes mixed-radix AllGather tasks; no follow-up-eligible issue was identified.
src/coll_patterns/sra_knomial.h	Initializes exact mixed-radix AllGather patterns while preserving the legacy single-radix path.
src/coll_patterns/recursive_knomial.h	Advances the active radix between mixed-schedule phases.
src/utils/ucc_parser.c	Adds parsing, printing, cloning, and release support for ranged radix schedules.
src/utils/ucc_datastruct.c	Adds deep-copy and destruction support for radix-schedule range entries.
test/gtest/coll/test_knomial_schedule.cc	Covers legacy fixed-radix behavior and exact mixed-radix peer and segment layouts.
test/gtest/utils/test_parser.cc	Covers valid, invalid, ranged, formatted, overlong, and cloned schedule configurations.

_{Reviews (4): Last reviewed commit: "TL/UCP: address follow-up mixed-radix re..." | Re-trigger Greptile}

Sergei-Lebedev · 2026-08-17T09:14:22Z

/build

Sergei-Lebedev · 2026-08-20T13:05:53Z

     UCC_CONFIG_TYPE_UINT_RANGED},

+    {"ALLGATHER_KN_MIXED_RADICES", "",
+     "Optional exact mixed-radix schedule for knomial allgather, for example "


i would maybe change semantic here, instead of exact decompostion consider it as allowed factors in decomposition so it generalises to any team size in a run.

don't we want the option for the user to provide the exact order ? (between 8x6 and 6x8 for example) I'm thinking that maybe the user would for example want to start with the first radix being the number of ranks in one node.

In the second PR, I provide an auto tuner that selects the radices and the order anyways but if the user chooses to select, I was thinking we ggive him "full control"

svcnbu-swx-hpcx · 2026-08-20T16:03:26Z

🤖 CI Triage Agent — Linter · commit d17161bf

TL;DR: The Linter (clang-tidy) job failed because allgather_knomial.c:250 contains a dead store — radix = p->radix; is assigned but never read in the ALLGATHER branch — which clang-tidy's clang-analyzer-deadcode.DeadStores check flags as an error (exit 125). Remove the dead assignment.

Full analysis

Summary: clang-tidy failed with 1 warning-treated-as-error: "Value stored to 'radix' is never read" in allgather_knomial.c.

Root cause: In ucc_tl_ucp_allgather_knomial_start, the ALLGATHER branch reassigns radix = p->radix; (line 250) after ucc_kn_ag_pattern_init, but nothing in that branch subsequently reads radix. radix is only consumed in the ALLGATHERV/else branches, which use the value set at initialization (line 234) and don't depend on line 250. This makes line 250 a dead store, which clang-analyzer-deadcode.DeadStores treats as an error under -warnings-as-errors, so the run-clang-tidy step counts an error: and exits 125.

Implicated commit: d17161b — "TL/UCP: address mixed-radix review" by jeffnvidia (the exact commit under test); the mixed-radix pattern work in ec51644 is the origin of the pattern-init reshuffle.

File: src/components/tl/ucp/allgather/allgather_knomial.c:250

Suggested fix: Delete the dead assignment radix = p->radix; on line 250. If the intent was for later code in the ALLGATHER branch to use the pattern-selected radix, then actually use it (e.g. pass it where needed); otherwise removing the line clears the clang-tidy error. If retaining it for symmetry/readability is desired, annotate with // NOLINT(clang-analyzer-deadcode.DeadStores), but simple removal is cleanest.

Related: PR #1327 (this change); prior mixed-radix PRs touching this file (#1176, #1072). No existing issue found for this specific warning.

svcnbu-swx-hpcx · 2026-08-20T16:07:41Z

🤖 CI Triage Agent — Lint (ROCm) · commit d17161bf

TL;DR: The Lint (ROCm) job failed because clang-tidy (run with -warnings-as-errors) flagged a dead store at allgather_knomial.c:250 — radix = p->radix; is assigned but never read afterward. Remove that unused assignment (or use the variable) to fix the lint error.

Full analysis

Summary: clang-tidy clang-analyzer-deadcode.DeadStores error caused the Lint job to exit with code 125 (the build itself succeeded).

Root cause: In ucc_tl_ucp_allgather_knomial_start, the UCC_COLL_TYPE_ALLGATHER branch assigns radix = p->radix; at line 250, but radix is not read again in that branch (only offset/rbuf are used, then the branch ends). clang-analyzer reports "Value stored to 'radix' is never read," and because the CI step counts any error: line and exits 125, the lint gate fails. This dead store was introduced by the new mixed-radix allgather logic, where ucc_kn_ag_pattern_init computes the effective radix into p->radix.

Implicated commit: ec516448 (jeffnvidia, "TL/UCP: add exact mixed-radix allgather"), on the current head d17161bf (jeffnvidia, "TL/UCP: address mixed-radix review").

File: src/components/tl/ucp/allgather/allgather_knomial.c:250

Suggested fix: Remove the now-unused radix = p->radix; line in the ALLGATHER branch. If the intent was to make the effective (possibly clamped) radix visible for later use, then it must actually be used — but since the subsequent code in that branch doesn't reference radix, the clean fix is to delete line 250. If radix is unused after ucc_kn_ag_pattern_init in all branches, also confirm the local isn't left otherwise unused.

Related: PR #1327 (this PR, jmahou/ucc-mixed-radix-clean). No prior matching issue found.

svcnbu-swx-hpcx · 2026-08-20T16:08:08Z

🤖 CI Triage Agent — Linter-NVIDIA · commit d17161bf

TL;DR: The Linter-NVIDIA clang-tidy job failed because allgather_knomial.c:250 stores p->radix into the local radix variable that is never read again, tripping clang-analyzer-deadcode.DeadStores (treated as error). Remove the dead radix = p->radix; assignment.

Full analysis

Summary: clang-tidy-17 static analysis failed with 1 warning treated as error: dead store to radix in allgather_knomial.c, causing the job to exit 125.

Root cause: In ucc_tl_ucp_allgather_knomial_start(), the UCC_COLL_TYPE_ALLGATHER branch calls ucc_kn_ag_pattern_init(...) and then executes radix = p->radix; (line 250). The local radix is not used anywhere after this assignment in that branch (the code uses p members, offset, and rbuf), so the store is dead. clang-analyzer flags it as clang-analyzer-deadcode.DeadStores, and the linter workflow treats warnings as errors (1 warning treated as error), producing exit code 125. This was introduced by the mixed-radix refactor on this branch, where the pattern-init API changed and left the reassignment vestigial.

Implicated commit: d17161b — jeffnvidia, "TL/UCP: address mixed-radix review" (HEAD of PR #1327; the dead-store pattern originates in the mixed-radix series starting ec51644).

File: src/components/tl/ucp/allgather/allgather_knomial.c:250

Suggested fix: Delete the dead assignment on line 250 (radix = p->radix;). Since radix is already initialized at line 234 from task->allgather_kn.p.radix and is only read in the ALLGATHERV/AGX branches (lines 271, 296, 300), removing line 250 clears the warning without changing behavior. If the intent was to refresh radix after ucc_kn_ag_pattern_init mutates p->radix, verify whether any later ALLGATHER-branch code actually needs it — currently none does — otherwise the removal is correct.

Related: PR #1327 (jmahou/ucc-mixed-radix-clean); prior mixed-radix commit ec51644.

jeffnvidia mentioned this pull request Jul 12, 2026

TL/UCP: select exact allgather radix schedules #1328

Open

jeffnvidia force-pushed the jmahou/ucc-mixed-radix-clean branch 3 times, most recently from 2dab095 to 828683a Compare July 15, 2026 09:07

jeffnvidia marked this pull request as ready for review July 15, 2026 09:31

jeffnvidia force-pushed the jmahou/ucc-mixed-radix-clean branch from 367c6f9 to 828683a Compare July 15, 2026 12:48

jeffnvidia force-pushed the jmahou/ucc-mixed-radix-clean branch 4 times, most recently from da58ec5 to d737a5a Compare July 19, 2026 14:26

jeffnvidia added 2 commits July 19, 2026 20:20

TL/UCP: add exact mixed-radix allgather

ec51644

Add explicit per-phase radix schedules to TL/UCP K-nomial AllGather while preserving the fixed-radix and AllGatherV paths. Parse and validate exact schedules through the existing team configuration.

TEST: cover mixed-radix allgather schedules

94d3bd9

Cover schedule parsing, legacy fixed-radix behavior, and exact mixed-radix peer and segment layouts.

jeffnvidia force-pushed the jmahou/ucc-mixed-radix-clean branch from d737a5a to 94d3bd9 Compare July 19, 2026 17:24

Sergei-Lebedev added the Ready-for-Review label Aug 17, 2026

Sergei-Lebedev reviewed Aug 20, 2026

View reviewed changes

TL/UCP: address mixed-radix review

d17161b

jeffnvidia added 2 commits August 20, 2026 19:11

TL/UCP: remove dead radix assignment

155b53b

TL/UCP: address follow-up mixed-radix review

8e47d8d

Navigation Menu

Uh oh!

TL/UCP: add exact mixed-radix allgather schedules - #1327

TL/UCP: add exact mixed-radix allgather schedules#1327
jeffnvidia wants to merge 5 commits into
openucx:masterfrom
jeffnvidia:jmahou/ucc-mixed-radix-clean

jeffnvidia commented Jul 12, 2026 •

edited

Loading

Uh oh!

jeffnvidia commented Jul 15, 2026

Uh oh!

greptile-apps Bot commented Aug 17, 2026 •

edited

Loading

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Sergei-Lebedev commented Aug 17, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Sergei-Lebedev Aug 20, 2026

Uh oh!

jeffnvidia Aug 20, 2026

Uh oh!

svcnbu-swx-hpcx commented Aug 20, 2026

Uh oh!

svcnbu-swx-hpcx commented Aug 20, 2026

Uh oh!

svcnbu-swx-hpcx commented Aug 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

jeffnvidia commented Jul 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Performance evidence

Validation

Uh oh!

jeffnvidia commented Jul 15, 2026

Uh oh!

greptile-apps Bot commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Sergei-Lebedev commented Aug 17, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Sergei-Lebedev Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

jeffnvidia Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

svcnbu-swx-hpcx commented Aug 20, 2026

Uh oh!

svcnbu-swx-hpcx commented Aug 20, 2026

Uh oh!

svcnbu-swx-hpcx commented Aug 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jeffnvidia commented Jul 12, 2026 •

edited

Loading

greptile-apps Bot commented Aug 17, 2026 •

edited

Loading